fix(azure): strip model from request body for deployment-based endpoints#2910
fix(azure): strip model from request body for deployment-based endpoints#2910giulio-leone wants to merge 1 commit intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Azure deployment-routing requests by removing the model field from the JSON body after it’s used to rewrite the URL to /deployments/{model}/..., preventing Azure configurations from rejecting requests due to body/URL model mismatches.
Changes:
- Update Azure client
_build_requestto rewrite the URL usingmodeland then rebuildjson_datawithoutmodel. - Add a regression test asserting the rewritten deployment URL and that
modelis not present in the request body.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/openai/lib/azure.py |
Strips model from JSON body when implicitly rewriting deployment-based endpoint URLs. |
tests/lib/test_azure.py |
Adds a regression test ensuring model is removed from the request body for an implicit deployment endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…, parametrized test - Replace type: ignore[union-attr] with proper cast() type narrowing - Assert full URL instead of substring match in deployment test - Add parametrized test for /chat/completions endpoint - Move import json to module scope for consistency Refs: openai#2910
giulio-leone
force-pushed
the
fix/azure-strip-model-from-deployment-body
branch
from
f6d0357 to
8622ec9
Compare
…ints Replace two separate single-endpoint tests with one parametrized test covering all 8 deployment endpoints. json import was already at module scope and type: ignore was already removed. Refs: openai#2910
giulio-leone
force-pushed
the
fix/azure-strip-model-from-deployment-body
branch
from
33c8eb9 to
41a8235
Compare
|
Intervention note for this PR: Current blocker appears to be missing CI execution rather than failing jobs:
Suggested unblock sequence:
If useful, I can run a follow-up status sweep as soon as checks are attached. |
|
This PR is ready for review — all CI checks pass, no merge conflicts, and all review threads have been resolved. Ready to merge when approved. 🚀 |
giulio-leone
force-pushed
the
fix/azure-strip-model-from-deployment-body
branch
from
41a8235 to
45fa03d
Compare
|
Hi! 👋 Gentle ping — this PR is rebased, CI passes, and ready for review. Happy to address any feedback. Thanks! |
|
Hi! Gentle ping — this PR is rebased, CI passes, and ready for review. Happy to address any feedback. Thanks! |
Summary
When using implicit deployments (model name as deployment), the Azure client correctly rewrites the URL to include
/deployments/{model}/, but leaves themodelparameter in the request body. Some Azure configurations reject requests where the body model doesn't match the actual model name behind the deployment.Changes
_build_request, after extractingmodelfromjson_datafor URL routing, create a new dict withoutmodelinstead of leaving it in the bodypop()to avoid mutating the sharedjson_datareference (model_copyis shallow), which preserves correct retry behaviorTest
Added
test_implicit_deployment_strips_model_from_bodywith two parametrized cases:/chat/completions(deployment endpoint)/images/generations(deployment endpoint)Both verify the URL contains
/deployments/{model}/and the request body does NOT containmodel.All 53 existing Azure tests pass.
Fixes #2892